feat(platform): pluggable credential resolvers — remove the 1Password default by jamaynor · Pull Request #3841 · mvanhorn/cli-printing-press

jamaynor · 2026-07-29T11:17:59Z

What

Every printed CLI currently ships a 1Password dependency whether or not its operator uses 1Password:

platform_profile.go.tmpl emits an OnePasswordResolver that shells out to 1password-pp-cli secrets read --reveal
ValidateCredentialReference rejects any reference not beginning with op://
platform_cli.go.tmpl's platformResolverFactory returns that resolver as the only option

The CredentialResolver interface was already the right abstraction — it was just bypassed by a hardcoded validator and a single hardcoded implementation. This puts a registry behind it:

RegisterCredentialResolver(scheme, resolver) — scheme and resolver register together, so a scheme the validator accepts but no resolver handles cannot exist
ValidateCredentialReference is registry-driven; with nothing registered every reference is rejected with an error saying so
RegistryResolver dispatches by scheme and is what the generated CLI uses; no vendor is privileged
New spec field auth.credential_resolvers: [file|bitwarden|onepassword], defaulting to [file] — the one resolver that depends on nothing installed. 1Password remains available as an explicit opt-in
Unknown resolver names are a spec validation error naming the catalog, not a silent skip

Notes for review

CredentialScheme.Validate exists because a filesystem path is not a sequence of opaque identifiers: file:///etc/key splits to a leading empty component, so the generic component rules would reject every absolute path
Test fixtures now come from a credentialRefFixture template function instead of hardcoded op:// strings — hardcoding one vendor's syntax in the test templates is part of how the original assumption stayed invisible
Subprocess-backed resolvers share runCredentialCommand, which enforces: no credential value ever enters argv, and provider stderr never enters an error (a secret manager may echo the resolved value while reporting an unrelated failure)
Second commit is an unrelated pre-existing fix: the read-only-dir save test fails under root (CAP_DAC_OVERRIDE bypasses the DAC check); it now skips as root, mirroring its existing Windows skip. Happy to split it out if preferred

Behaviour change

A reprint of a CLI that used op:// references rejects them unless its spec opts into onepassword. In practice nothing is affected today: registerPlatformSource is never called by any printed CLI, so this path is dead code everywhere — which is also why the hard dependency went unnoticed.

Verification

internal/spec suite green including 9 new tests
Full internal/generator suite run to completion (-timeout 45m): no failures attributable to this change (one pre-existing root-environment failure, fixed in the second commit)
Three end-to-end prints of a real spec — default, [bitwarden, file], [onepassword] — each passing the generator's own build/vet/test/bundle gates; the default print contains zero 1Password references

🤖 Generated with Claude Code

…word default Every printed CLI shipped a 1Password dependency whether or not its operator used 1Password. platform_profile.go.tmpl emitted an OnePasswordResolver that shells out to `1password-pp-cli secrets read --reveal`; ValidateCredentialReference rejected any reference not beginning with op://; and platform_cli.go.tmpl's platformResolverFactory returned that resolver as the only option. The net effect was that the entire client-profile mechanism was unusable without 1Password installed, and every SourceProfile in every printed CLI was implicitly a 1Password profile. The CredentialResolver interface already existed and was already the right abstraction — it was simply bypassed by a hardcoded validator and a single hardcoded implementation. This change puts a registry behind it. - CredentialScheme declares a reference syntax; RegisterCredentialResolver takes a scheme AND its resolver together, so a scheme the validator accepts but no resolver handles cannot exist. That combination is a reference that passes validation and then fails at use, which is the worst time to find out. - ValidateCredentialReference is registry-driven. No scheme is built in. With none registered, every reference is rejected and the error says exactly that — correct for a CLI never configured to reach a secret manager, and strictly better than silently binding to whichever vendor was hardcoded. - RegistryResolver dispatches by scheme and is what platformResolverFactory now returns. No manager is privileged. - auth.credential_resolvers selects which resolvers compile in. Omitted, it defaults to [file] — the one resolver that depends on nothing installed. 1Password remains available as an explicit opt-in for anyone who does use it. - Three resolvers ship: file (file://), bitwarden (bws://), onepassword (op://). Adding a fourth is a template plus a catalog entry; the validator, registry and dispatcher do not change. Two things fell out of building it that are worth keeping: CredentialScheme.Validate exists because a filesystem path is not a sequence of opaque identifiers. "file:///etc/key" splits to a leading EMPTY component, so the generic component rules rejected every absolute path — that is, every valid file reference. Path-shaped schemes supply their own rules rather than contorting the generic ones. Test fixtures are now generated via a credentialRefFixture template function rather than hardcoding op:// strings. Hardcoding one vendor's syntax in the test templates is part of how the original assumption got baked in and stayed invisible: the conformance tests could only pass under 1Password. Subprocess-backed resolvers share runCredentialCommand, which enforces the two rules that were previously restated per vendor: no credential value ever enters argv, and the provider's stderr never enters an error (a secret manager may echo the resolved value while reporting an unrelated failure, and error strings reach logs and run receipts). Behaviour change: a reprint of an existing CLI that used op:// references will reject them unless its spec opts into `onepassword`. In practice nothing is affected today — registerPlatformSource is never called by any printed CLI, so this entire path is dead code everywhere, which is also why the 1Password dependency went unnoticed. Verified by printing a real spec three ways: default (emits resolver_file.go only, zero 1Password references anywhere in the tree), credential_resolvers: [bitwarden, file], and [onepassword]. All three pass the generator's own build, vet, test and bundle gates. An unknown resolver name is rejected at spec validation with the catalog listed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PieLzEBrxVUbR4HauaTG3u

TestConfigSaveFailureLeavesOriginalParseable makes save() fail by chmod-ing the config directory to 0500 and asserting the write is refused. Root ignores that: CAP_DAC_OVERRIDE bypasses the DAC check, so the temp file is created anyway, save() returns nil, and the test fails for a reason unrelated to the code it covers. Verified rather than assumed, in both directions: as uid 0 a write into a 0500 directory succeeds, and as a normal user the same write is refused. So the premise holds everywhere except root, and the skip is environment-specific rather than a blanket disable — the assertion still runs in CI and on developer machines. This mirrors the Windows skip already at the top of the same test: the mechanism the test depends on does not exist in this environment. The alternative — loosening the assertion to tolerate a nil error — would delete the coverage for everyone, which is the wrong trade for an environment quirk. Pre-existing on main; unrelated to the credential-resolver change in the preceding commit, and kept separate for that reason. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PieLzEBrxVUbR4HauaTG3u

mergify · 2026-07-29T11:18:43Z

Merge Protections

🔴 2 of 2 protections blocking · waiting on 👀 reviews, 🤖 CI and 🙋 you

	Protection	Waiting on
🔴	require-ready-label-and-ci	👀 reviews, 🤖 CI and 🙋 you
🔴	🚦 Auto-queue	🙋 you

🔴 require-ready-label-and-ci

Waiting for

This rule is failing.

🔴 🚦 Auto-queue

Waiting for

label = ready-to-merge

This rule is failing.

When all merge protections are satisfied and these conditions match, this pull request will be queued automatically.

label = ready-to-merge

greptile-apps · 2026-07-29T11:21:30Z

+	// Surrounding whitespace is stripped so a file written with a trailing
+	// newline — which every editor and every `echo` produces — works.
+	value := strings.TrimSpace(string(data))
+	if value == "" {
+		return nil, fmt.Errorf("credential file %s is empty", path)
+	}
+	return []byte(value), nil


File resolver mutates credentials

When a file-backed credential legitimately begins or ends with whitespace, strings.TrimSpace removes those significant bytes, causing the generated CLI to authenticate with a different value and reject an otherwise valid credential.

Suggested change

// Surrounding whitespace is stripped so a file written with a trailing

// newline — which every editor and every `echo` produces — works.

value := strings.TrimSpace(string(data))

if value == "" {

return nil, fmt.Errorf("credential file %s is empty", path)

}

return []byte(value), nil

// Remove one editor-added line ending without changing other credential

// bytes, since leading or trailing whitespace may be significant.

value := strings.TrimSuffix(string(data), "\n")

value = strings.TrimSuffix(value, "\r")

if value == "" {

return nil, fmt.Errorf("credential file %s is empty", path)

}

return []byte(value), nil

greptile-apps · 2026-07-29T11:21:34Z

Greptile Summary

Adds spec-selectable credential resolvers and makes file-backed credentials the vendor-neutral default.

Introduces registry-driven credential reference validation and dispatch in generated CLIs.
Adds file, Bitwarden, and opt-in 1Password resolver templates plus shared subprocess handling.
Adds resolver catalog validation and resolver-aware generated test fixtures.
Documents auth.credential_resolvers and updates generated conformance tests.
Skips a POSIX permission test when running as root.

Confidence Score: 3/5

The file resolver’s credential mutation should be fixed before merging because it can make valid file-backed credentials fail authentication.

The new default resolver applies strings.TrimSpace to credential contents, changing valid leading or trailing whitespace bytes before they reach the authentication path.

Files Needing Attention: internal/generator/templates/platform_resolver_file.go.tmpl

Important Files Changed

Filename	Overview
internal/generator/templates/platform_profile.go.tmpl	Replaces hardcoded 1Password validation and resolution with a synchronized scheme-and-resolver registry.
internal/generator/templates/platform_resolver_file.go.tmpl	Adds the default file resolver, but its blanket whitespace trimming can alter valid credential values.
internal/generator/templates/platform_resolver_bitwarden.go.tmpl	Adds ID-based and project/key-based Bitwarden Secrets Manager resolution with duplicate-key rejection.
internal/generator/templates/platform_resolver_onepassword.go.tmpl	Moves 1Password support into an explicitly selected resolver template.
internal/spec/credential_resolvers.go	Defines the resolver catalog, default selection, template mapping, validation, and synthetic fixture references.
internal/generator/generator.go	Emits only the credential resolver templates selected by the effective auth configuration.
internal/spec/spec.go	Adds the resolver selection field to AuthConfig and validates catalog names during spec validation.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
    S[auth.credential_resolvers] --> T[Selected resolver templates]
    T --> I[Resolver init registration]
    R[Credential reference] --> V[Registry validation]
    V --> D[RegistryResolver dispatch]
    I --> V
    D --> F[File resolver]
    D --> B[Bitwarden resolver]
    D --> O[1Password resolver]
    F --> C[Resolved credential bytes]
    B --> C
    O --> C

_{Reviews (1): Last reviewed commit: "test(generator): skip the read-only-dir ..." | Re-trigger Greptile}

…resolvers Merged to the fork's main so local prints and installs carry the fix while upstream PR mvanhorn#3841 awaits review. The upstream merge remains the maintainer's call. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PieLzEBrxVUbR4HauaTG3u

root and others added 2 commits July 29, 2026 00:06

greptile-apps Bot reviewed Jul 29, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(platform): pluggable credential resolvers — remove the 1Password default - #3841

feat(platform): pluggable credential resolvers — remove the 1Password default#3841
jamaynor wants to merge 2 commits into
mvanhorn:mainfrom
jamaynor:feat/pluggable-credential-resolvers

jamaynor commented Jul 29, 2026

Uh oh!

mergify Bot commented Jul 29, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot Jul 29, 2026

Uh oh!

greptile-apps Bot commented Jul 29, 2026

Important Files Changed

Flowchart

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

jamaynor commented Jul 29, 2026

What

Notes for review

Behaviour change

Verification

Uh oh!

mergify Bot commented Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge Protections

🔴 require-ready-label-and-ci

🔴 🚦 Auto-queue

Uh oh!

greptile-apps Bot Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot commented Jul 29, 2026

Greptile Summary

Confidence Score: 3/5

Important Files Changed

Flowchart

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

mergify Bot commented Jul 29, 2026 •

edited

Loading